fix: frozen string literal issue for ruby 3.4.0 #137
Conversation
chaadow
changed the title
lib/ruby_http_client.rb
Outdated
| url = [add_version(''), *@url_path].join('/') | ||
| url = build_query_params(url, query_params) if query_params | ||
| url = [add_version(+''), *@url_path].join('/') | ||
| url = build_query_params(+url, query_params) if query_params |
There was a problem hiding this comment.
I don't think this url is frozen because it is not “string literal” 🤔
| url = build_query_params(+url, query_params) if query_params | |
| url = build_query_params(url, query_params) if query_params |
There was a problem hiding this comment.
It is. The line before does a join('/')
So it creates a string literal
There was a problem hiding this comment.
@childish-sambino, @rakatyal, @garethpaul, @tmimura39. Sorry to bother you but could look at this one?
There was a problem hiding this comment.
I agree with @tmimura39 - url is a string, but it's not a string literal. As proof:
irb(main):002> a = ["foo", "bar"]
=> ["foo", "bar"]
irb(main):003> a.map! { it << " world" }
(irb):3:in 'block in <top (required)>': can't modify frozen String: "foo" (FrozenError)
from (irb):3:in 'Array#map!'
from (irb):3:in '<main>'
from <internal:kernel>:168:in 'Kernel#loop'
from /Users/chrishowlett/.local/share/mise/installs/ruby/3.4.1/lib/ruby/gems/3.4.0/gems/irb-1.15.1/exe/irb:9:in '<top (required)>'
from /Users/chrishowlett/.local/share/mise/installs/ruby/3.4.1/bin/irb:25:in 'Kernel#load'
from /Users/chrishowlett/.local/share/mise/installs/ruby/3.4.1/bin/irb:25:in '<main>'
irb(main):004> a.join("/") << "world"
=> "foo/barworld"
Another bump for getting this merged - we depend on this gem and it's blocking our ability to enable frozen literals in 3.4, and will (probably) Just Break from Ruby 4
There was a problem hiding this comment.
You're right! Idk why I thought it was back then. There are so many recent methods that create frozen strings
For example "false.to_s" creates a frozen string
To prepare for ruby 3.4.0 and making strings frozen by default, we add `+` operator to make the string mutable
Fixes
Fixes #136
To prepare for ruby 3.4.0 and making strings frozen by default, I've stumbled upon these string mutations in my specs while trying to send an email
future of frozen string literals in ruby : https://gist.github.com/fxn/bf4eed2505c76f4fca03ab48c43adc72
similar to fog/fog-aws#709
Checklist
If you have questions, please file a support ticket.